Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define the imap_processing spice structure #733

Conversation

subagonsouth
Copy link
Contributor

Add files defining the structure of the spice package within imap_processing

Change Summary

Overview

This PR adds some empty files which define how SPICE related functions will be organized.

New Dependencies

None

New Files

  • imap_processing/spice/geometry.py
    • Functions for computing geometry using SPICE.
  • imap_processing/spice/kernels.py
    • Functions for furnishing and tracking SPICE kernels.
  • imap_processing/spice/time.py
    • Time conversion functions that rely on SPICE.
  • imap_processing/tests/spice/test_geometry.py
    • Tests coverage for imap_processing/spice/geometry.py
  • imap_processing/tests/spice/test_kernels.py
    • Tests coverage for imap_processing/spice/kernels.py
  • imap_processing/tests/spice/test_time.py
    • Tests coverage for imap_processing/spice/time.py

Deleted Files

None

Updated Files

None

Testing

None

Closes #732

@subagonsouth subagonsouth added the SPICE Related to SPICE label Aug 6, 2024
@subagonsouth subagonsouth self-assigned this Aug 6, 2024
Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about SPICE to know that this is the directory structure we want or not. Seems reasonable though.

My only comment is that I would actually prefer slightly larger PRs that make use of these files :) Or maybe one-by-one adding the kernels.py and test_kernels.py in separate PRs. But, I do appreciate the thought in trying to break it up.

@greglucas
Copy link
Collaborator

As a quick comment: we have some work already done in tools/ https://github.com/IMAP-Science-Operations-Center/imap_processing/tree/dev/tools/spice
where does that work fit into this? Are we going to be merging everything together and only having things in the deployed package, or are those good examples to keep separate?

@subagonsouth
Copy link
Contributor Author

@greglucas

As a quick comment: we have some work already done in tools/ https://github.com/IMAP-Science-Operations-Center/imap_processing/tree/dev/tools/spice
where does that work fit into this? Are we going to be merging everything together and only having things in the deployed package, or are those good examples to keep separate?

Right... I am not sure about why all of that code was located there. It looks to me like some of that code can be pulled in to the imap_processing package. Is the tools directory intended to contain development tools that are not needed/wanted in the deployed package?

My only comment is that I would actually prefer slightly larger PRs that make use of these files :) Or maybe one-by-one adding the kernels.py and test_kernels.py in separate PRs. But, I do appreciate the thought in trying to break it up.

As I was working on the kernel handling decorator, I realized that I am working on one piece, Laura is working on another and Veronica is working on a third. I thought would be prudent to define an organization for SPICE related functions that has worked on other projects so that the three of us can be consistent. That is the main reason that I broke this PR out and added files with nothing in them.

@greglucas
Copy link
Collaborator

That all makes sense to me and you still have my approval :)

Is the tools directory intended to contain development tools that are not needed/wanted in the deployed package?

Correct. Not bringing unnecessary dependencies.

@subagonsouth subagonsouth merged commit cb26199 into IMAP-Science-Operations-Center:dev Aug 7, 2024
17 checks passed
@subagonsouth subagonsouth deleted the 732-create-module-struture-for-spice-functions branch August 7, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SPICE Related to SPICE
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Create module struture for spice functions
2 participants